Support keys and indexes that traverse complex-type properties#38192
Support keys and indexes that traverse complex-type properties#38192AndriySvyryd wants to merge 1 commit into
Conversation
55029ef to
8c4181d
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends EF Core’s model-building and runtime pipeline to support keys and indexes whose elements traverse non-collection complex properties (and, where applicable, target a complex property itself when mapped to JSON). It widens index metadata to expose property-or-complex-property entries, updates validation/conventions accordingly, and flows these shapes through runtime/compiled models, migrations, and provider-specific behaviors.
Changes:
- Widen index metadata (
*.Properties) from scalar properties toIPropertyBase/IReadOnlyPropertyBaseacross read-only/mutable/convention/runtime layers. - Add conventions + validation for complex-property chains in keys/indexes (including new user-facing errors).
- Update runtime model, compiled model generation/scaffolding, relational mapping, and provider logic (SQL Server/Cosmos) to understand the new shapes; extend tests/baselines.
Reviewed changes
Copilot reviewed 63 out of 66 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.Tests/Metadata/Internal/InternalEntityTypeBuilderTest.cs | Adjusts index lookup assertions for widened index property metadata. |
| test/EFCore.Tests/Metadata/Internal/EntityTypeTest.cs | Updates index removal tests to account for IPropertyBase-typed index properties. |
| test/EFCore.Tests/Metadata/Conventions/ConventionDispatcherTest.cs | Updates convention test to remove indexes using the new property-base shape. |
| test/EFCore.Tests/Infrastructure/ModelValidatorTest.cs | Adds model-validation coverage for keys/indexes traversing nullable complex properties. |
| test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs | Adds SQL Server IncludeProperties coverage for dotted paths into complex types. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/ComplexTypes/PrincipalBaseEntityType.cs | Updates compiled-runtime baseline to include an index traversing a complex property. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/Baselines/ComplexTypes/DbContextModelBuilder.cs | Updates relational-model baseline to include the new table index mapping. |
| test/EFCore.Specification.Tests/Scaffolding/CompiledModelTestBase.cs | Adds model-building + assertions for an index traversing a complex property in compiled models. |
| test/EFCore.Specification.Tests/Query/AdHocComplexTypeQueryTestBase.cs | Adds functional CRUD/batch tests for indexes/alternate keys traversing complex properties. |
| test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.NonGeneric.cs | Updates non-generic model-builder test helpers to emit dotted member chains. |
| test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.Inheritance.cs | Updates inheritance tests to compare index properties via scalar subset when needed. |
| test/EFCore.Specification.Tests/ModelBuilding/ModelBuilderTest.ComplexType.cs | Adds model-building tests for alternate keys/indexes on complex-type properties (lambda + dotted strings). |
| test/EFCore.Relational.Tests/Metadata/RelationalModelTest.cs | Adds relational-model assertions for unique constraints/table indexes based on complex-type properties. |
| test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationsGeneratorTest.ModelSnapshot.cs | Updates snapshot tests for widened index properties (casts for relational extensions). |
| src/EFCore/Properties/CoreStrings.resx | Adds new CoreStrings messages for complex key/index validation failures. |
| src/EFCore/Properties/CoreStrings.Designer.cs | Adds corresponding strongly-typed accessors for new CoreStrings messages. |
| src/EFCore/Metadata/RuntimePropertyBase.cs | Moves runtime “containing indexes” storage up to RuntimePropertyBase. |
| src/EFCore/Metadata/RuntimeProperty.cs | Removes runtime index list from RuntimeProperty (now on RuntimePropertyBase). |
| src/EFCore/Metadata/RuntimeKey.cs | Fixes DeclaringEntityType resolution for key properties declared on complex types. |
| src/EFCore/Metadata/RuntimeIndex.cs | Widens runtime index properties to RuntimePropertyBase and adapts value-factory creation. |
| src/EFCore/Metadata/RuntimeEntityType.cs | Widens unnamed-index dictionary key type to IReadOnlyPropertyBase list and updates Find/Add signatures. |
| src/EFCore/Metadata/Internal/TypeBase.cs | Introduces internal ContainingEntityType abstraction for complex/entity types. |
| src/EFCore/Metadata/Internal/PropertyListComparer.cs | Widens comparer to IReadOnlyPropertyBase lists (used in index dictionaries). |
| src/EFCore/Metadata/Internal/PropertyBase.cs | Moves “containing indexes” tracking/helpers up to PropertyBase. |
| src/EFCore/Metadata/Internal/Property.cs | Removes index list/helpers from Property (now on PropertyBase). |
| src/EFCore/Metadata/Internal/Key.cs | Fixes DeclaringEntityType resolution for key properties declared on complex types. |
| src/EFCore/Metadata/Internal/InternalTypeBaseBuilder.cs | Adds member-chain property creation and introduces GetActualPropertyBases for index attachment. |
| src/EFCore/Metadata/Internal/InternalIndexBuilder.cs | Updates Attach() to resolve “actual” property bases instead of only scalar properties. |
| src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs | Adds member-chain overloads for key/index APIs and broadens HasIndex plumbing to PropertyBase. |
| src/EFCore/Metadata/Internal/Index.cs | Widens index constructor/properties to PropertyBase and updates runtime/value-factory behavior. |
| src/EFCore/Metadata/Internal/EntityType.cs | Widens index storage/removal/find APIs to IReadOnlyPropertyBase lists; adds ContainingEntityType override. |
| src/EFCore/Metadata/Internal/ComplexType.cs | Implements ContainingEntityType via base TypeBase contract. |
| src/EFCore/Metadata/IReadOnlyIndex.cs | Widens read-only index properties to IReadOnlyPropertyBase. |
| src/EFCore/Metadata/IMutableIndex.cs | Widens mutable index properties to IMutablePropertyBase. |
| src/EFCore/Metadata/IIndex.cs | Widens index properties to IPropertyBase. |
| src/EFCore/Metadata/IConventionIndex.cs | Widens convention index properties to IConventionPropertyBase. |
| src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs | Updates runtime model creation for keys/indexes to resolve properties across complex chains. |
| src/EFCore/Metadata/Conventions/KeyOrIndexComplexPropertyConvention.cs | New convention: implicitly marks complex-property chains required when used in keys/indexes. |
| src/EFCore/Metadata/Conventions/Infrastructure/ProviderConventionSetBuilder.cs | Registers the new KeyOrIndexComplexPropertyConvention. |
| src/EFCore/Metadata/Conventions/ForeignKeyIndexConvention.cs | Updates index coverage checks for new property-base shapes (name-based comparison). |
| src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs | Switches key/index lambda parsing to member access chains (supports complex traversal). |
| src/EFCore/Infrastructure/ModelValidator.cs | Adds validation for index/key complex traversal (collections/nullable chain + shape checks). |
| src/EFCore/Extensions/Internal/ExpressionExtensions.cs | Adds GetMemberAccessChainList to extract chained member access paths from lambdas. |
| src/EFCore/EFCore.baseline.json | Updates API baseline for widened index property types and new convention/messages. |
| src/EFCore/Design/ICSharpHelper.cs | Adds Lambda generation overload for IPropertyBase (dotted paths for complex-type properties). |
| src/EFCore.SqlServer/Metadata/Internal/SqlServerIndexExtensions.cs | Updates Include column resolution to use dotted property-path lookup. |
| src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs | Updates include-column annotations to resolve dotted property paths. |
| src/EFCore.SqlServer/Metadata/Conventions/SqlServerIndexConvention.cs | Skips non-scalar index elements when building filters (adapts to property-base index shape). |
| src/EFCore.SqlServer/Infrastructure/Internal/SqlServerModelValidator.cs | Updates include-property validation and index-property iteration to handle property-base shapes. |
| src/EFCore.SqlServer/Extensions/SqlServerIndexBuilderExtensions.cs | Updates IncludeProperties lambda parsing to support complex-property chains (dotted paths). |
| src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs | Adjusts unique-index coverage check to use scalar subset of property-base index properties. |
| src/EFCore.Relational/Properties/RelationalStrings.resx | Adds relational validation message for complex properties mapped to a different table. |
| src/EFCore.Relational/Properties/RelationalStrings.Designer.cs | Adds corresponding strongly-typed accessor for new relational string. |
| src/EFCore.Relational/Metadata/Internal/RelationalModel.cs | Adds property-path lookup helpers and adapts index/constraint mapping to property-base shapes. |
| src/EFCore.Relational/Metadata/Internal/RelationalIndexExtensions.cs | Adapts column-name compatibility checks to scalar subset of index properties. |
| src/EFCore.Relational/Metadata/IColumnBase.cs | Fixes column mapping selection to account for complex-type property ownership via ContainingEntityType. |
| src/EFCore.Relational/Infrastructure/RelationalModelValidator.cs | Adds table-mapping validation for complex-property chains referenced by indexes; adapts property iteration. |
| src/EFCore.Relational/Extensions/RelationalIndexExtensions.cs | Adapts index naming/DB name logic to scalar subset of index properties. |
| src/EFCore.Relational/EFCore.Relational.baseline.json | Updates relational API baseline for new relational string. |
| src/EFCore.Relational/Design/Internal/RelationalCSharpRuntimeAnnotationCodeGenerator.cs | Emits dotted property paths for keys/indexes in compiled relational model generation. |
| src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs | Updates runtime model scaffolding to resolve index property bases across complex chains. |
| src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs | Regenerated T4 output (runtime/text templating version bump; access modifier change). |
| src/EFCore.Design/Properties/DesignStrings.resx | Reorders/adds design-time resource strings (migration-related messages). |
| src/EFCore.Design/Properties/DesignStrings.Designer.cs | Updates generated accessors to match resx changes. |
| src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs | Adapts vector/full-text index path extraction to widened index properties. |
| src/EFCore.Cosmos/Infrastructure/Internal/CosmosModelValidator.cs | Adapts Cosmos index validation to handle property-base index properties safely. |
Files not reviewed (3)
- src/EFCore.Design/Properties/DesignStrings.Designer.cs: Language not supported
- src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Language not supported
- src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 86 out of 89 changed files in this pull request and generated 3 comments.
Files not reviewed (3)
- src/EFCore.Design/Properties/DesignStrings.Designer.cs: Language not supported
- src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Language not supported
- src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported
Comments suppressed due to low confidence (1)
src/EFCore.Design/Scaffolding/Internal/CSharpEntityTypeGenerator.cs:432
- This file change makes CSharpEntityTypeGeneratorBase.GenerationEnvironment public (was protected), expanding the public surface area of EFCore.Design scaffolding APIs. This seems unrelated to the PR's described public API impact and is likely an accidental T4 regeneration artifact; consider reverting the accessibility change (or regenerating with consistent tooling) to avoid an unintended API change.
/// <summary>
/// The string builder that generation-time code is using to assemble generated output
/// </summary>
public System.Text.StringBuilder GenerationEnvironment
{
get
{
if ((this.generationEnvironmentField == null))
{
this.generationEnvironmentField = new global::System.Text.StringBuilder();
}
return this.generationEnvironmentField;
}
set
{
this.generationEnvironmentField = value;
}
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 94 changed files in this pull request and generated 5 comments.
Files not reviewed (3)
- src/EFCore.Design/Properties/DesignStrings.Designer.cs: Language not supported
- src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Language not supported
- src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported
5f9e7d1 to
e719275
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 94 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- src/EFCore.Design/Properties/DesignStrings.Designer.cs: Language not supported
- src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Language not supported
- src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported
| if (!propertyMap.TryGetValue(property, out var columnExpression)) | ||
| { | ||
| // The key property is declared on a complex type; navigate the complex-property chain | ||
| // from the entity to its declaring complex type and bind the property there. | ||
| var chain = new Stack<IComplexProperty>(); | ||
| for (var current = property.DeclaringType as IComplexType; | ||
| current != null; | ||
| current = current.ComplexProperty.DeclaringType as IComplexType) | ||
| { | ||
| chain.Push(current.ComplexProperty); | ||
| } | ||
|
|
||
| var shaper = (RelationalStructuralTypeShaperExpression)complexPropertyMap[chain.Pop()]; | ||
| var complexProjection = (StructuralTypeProjectionExpression)shaper.ValueBufferExpression; | ||
| while (chain.Count > 0) | ||
| { | ||
| shaper = (RelationalStructuralTypeShaperExpression)complexProjection.BindComplexProperty(chain.Pop()); | ||
| complexProjection = (StructuralTypeProjectionExpression)shaper.ValueBufferExpression; | ||
| } | ||
|
|
||
| columnExpression = complexProjection.BindProperty(property); | ||
| } |
There was a problem hiding this comment.
@roji I expect this will be replaced with your changes.
There was a problem hiding this comment.
Yeah looks non-ideal, but looks good enough for now.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 92 out of 95 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- src/EFCore.Design/Properties/DesignStrings.Designer.cs: Language not supported
- src/EFCore.Relational/Properties/RelationalStrings.Designer.cs: Language not supported
- src/EFCore/Properties/CoreStrings.Designer.cs: Language not supported
|
|
||
| if (index.Properties[0].GetVectorDistanceFunction() == null | ||
| || index.Properties[0].GetVectorDimensions() == null) | ||
| var firstIndexProperty = index.Properties[0] as IProperty; |
There was a problem hiding this comment.
| var firstIndexProperty = index.Properties[0] as IProperty; | |
| var firstVectorIndexProperty = index.Properties[0] as IProperty; |
| { | ||
| base.ValidateKey(key, logger); | ||
|
|
||
| foreach (var property in key.Properties) |
There was a problem hiding this comment.
Do we also need to do this for owned JSON?
There was a problem hiding this comment.
It will throw in model building if you attempt to use a property from an owned entity type (or any other entity type that's not in the hierarchy) in a key or index, independently of whether it's mapped to JSON
| <value>Default value '{value}' of type '{valueType}' cannot be set on property '{property}' of type '{propertyType}' in entity type '{entityType}'.</value> | ||
| </data> | ||
| <data name="IndexOnNonJsonComplexProperty" xml:space="preserve"> | ||
| <value>The index {indexProperties} on the entity type '{entityType}' cannot be configured because it is defined on the complex property '{property}', which is not mapped to a JSON column. Indexes on complex properties are only supported when the complex property is mapped to JSON.</value> |
There was a problem hiding this comment.
I'd make this message less conceptual and more concrete, saying that the (complex) property is mapped to multiple database columns via table splitting, and therefore can't have an index over it.
|
|
||
| if (complexProperties != null) | ||
| { | ||
| ValidateIndexOnComplexProperty(index, complexProperties, logger); |
There was a problem hiding this comment.
Maybe just have ValidateIndexProperty which accepts an IPropertyBase, and has validation for individual index properties (both scalar and complex)?
There was a problem hiding this comment.
The base implementation of ValidateIndexOnComplexProperty is to always throw, so most other factorings would require the derived class to duplicate some of the logic from this class
|
|
||
| if (index.IsUnique) | ||
| { | ||
| throw new InvalidOperationException( |
There was a problem hiding this comment.
I'm not sure it's quite correct to disallow unique indexes on JSON complex properties... In SQL Server an index on the JSON column means an index over all the individual properties inside (and uniqueness indeed isn't supported), but I think that in various other databases you can put an index on the JSON type just like any other database type (PostgreSQL, SQLite). So this may be a SQL Server-specific limitation.
There was a problem hiding this comment.
It's not about correctness, but scoping. We have special logic for unique indexes in the update pipeline and query that would need to be updated to support this. It would be significant effort for something that I doubt that is used much (or at all)
There was a problem hiding this comment.
OK, agree this would probably pretty niche and can always be done later if needed. Maybe good to add a quick comment in this kind of case to explain etc.
| { | ||
| foreach (var property in properties) | ||
| { | ||
| if (property.DeclaringType is not IConventionComplexType) |
There was a problem hiding this comment.
Does this mean that the JSON column itself (which is a property on the entity type, not on a complex type) can be non-required? i.e. only JSON properties need to be marked as required? Can you provide some context on this?
There was a problem hiding this comment.
The complex property itself cannot be part of the key, you need to list all of the contained properties as order is important
There was a problem hiding this comment.
And you cannot add a JSON column or JSON elements to a key
There was a problem hiding this comment.
OK. Technically in most databases a JSON column is just another column and can be used in the primary key (though probably not SQL Server which doesn't even allow comparing JSON types), but on the EF side I'm guessing this would require substantial support and is quite niche (like in the previous comment).
| } | ||
| } | ||
|
|
||
| private static bool IsStillReferenced(IConventionComplexProperty complexProperty) |
There was a problem hiding this comment.
Feels like we always address complexity by adding complexity...
| /// any release. You should only use it directly in your code with extreme caution and knowing that | ||
| /// doing so can result in application failures when updating to a new Entity Framework Core release. | ||
| /// </summary> | ||
| public ComplexPropertyNameComparer(IReadOnlyTypeBase typeBase) |
There was a problem hiding this comment.
Unless I'm mistaken, in SQL Server an index can't span both a JSON property and another property (JSON or otherwise) - though probably verify this. This would be a SQL Server restriction, not a universal one I think.
BTW it's worth mentioning that the precise meaning of "create an index on a JSON column" is a bit squishy and varies... In SQL Server this creates an index over the column means indexing everything inside the documents it contains (at least that's what CREATE JSON INDEX does), in PostgreSQL what happens exactly depends on the index "method" the user chooses (on SQLite it'd likely only speed up filters on the entire document). This is also very different from the normal meaning of defining an index over a (non-JSON) column.
In other words, I'm pointing out that the meaning of HasIndex() on JSON columns is going to be quite different from the meaning of the same method on non-JSON columns, and it will also vary quite considerably across databases (which is probably OK).
In any case, of course you can refine all of this when you actually tackle the SQL Server feature, no need to get everything 100% right in this PR.
There was a problem hiding this comment.
Yes, this will be covered in the next PR
Fixes #31246
Fixes #28605
Overview
This PR makes it possible to define keys and indexes whose key elements live on
a complex-type property rather than directly on the entity type (as well as
a complex property itself when mapped to JSON), The model, conventions,
validation, change tracking, runtime model, compiled model, migrations and
update pipeline have all been updated to handle these new key/index shapes
end-to-end.
What now works
HasKey,HasAlternateKeyandHasIndexaccept lambdas and dotted stringsthat traverse one or more non-collection complex properties.
entity properties now also applies along complex-property chains: a property
used in a key or index is required, and so are the complex properties on the
way to it.
compiled model code generator, migrations, change tracking, and SaveChanges
(including multi-row batches that exercise unique-value edges in the update
pipeline's topological sort).
Validation
New, user-facing validation errors are raised at model finalization for shapes
that cannot be supported:
Public API impact
The
Propertiescollection on the various index metadata interfaces(read-only, mutable, convention, runtime) has been widened from a list of
properties to a list of property-or-complex-property entries, so that callers
can observe the full shape of an index. This is the only public surface
change; all existing usages that work exclusively with scalar properties
continue to work unchanged.
The API baselines have been refreshed for the affected assemblies.
Tests
batched SaveChanges for entities whose keys/indexes traverse complex
properties.
relational-model tests have been extended to cover the new shapes.